[18.0][MIG] account_invoice_fiscal_position_update: Migration to 18.0 #1863
Conversation
|
LGTM |
|
@pedrobaeza Hi! I would like to ask you if it's something missing in this migration in order to be approve |
|
A PSC or maintainer should review it: https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#review |
|
Hi @rov-adhoc thanks for porting this module. could you rebase, to re build runboat. I'd like to know if the bug described here is still present in the 18.0 branch. (#1896) |
rov-adhoc
force-pushed
the
18.0-mig-account_invoice_fiscal_position_update
branch
from
25ebb65 to
0c47ca8
Compare
|
@legalsylvain thanks for your message. I was able to reproduce the same error in the 18.0 branch. Do you know if anyone is currently working on a fix for this issue? |
I don't know. I just raised this error, using the V16 module. Let's wait what maintainers and original developpers are saying about that bug. |
|
@StefanRijnhart HI! I just included your PR in this migration! thanks |
|
Thanks! Can you drop the merge commit? |
…on, tax mapping could be wrong set
Currently translated at 100,0% (4 of 4 strings) Translation: account-invoicing-11.0/account-invoicing-11.0-account_invoice_fiscal_position_update Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-11-0/account-invoicing-11-0-account_invoice_fiscal_position_update/nl_NL/
Currently translated at 100,0% (4 of 4 strings) Translation: account-invoicing-11.0/account-invoicing-11.0-account_invoice_fiscal_position_update Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-11-0/account-invoicing-11-0-account_invoice_fiscal_position_update/de/
Currently translated at 100.0% (4 of 4 strings) Translation: account-invoicing-12.0/account-invoicing-12.0-account_invoice_fiscal_position_update Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-12-0/account-invoicing-12-0-account_invoice_fiscal_position_update/de/
Currently translated at 100.0% (4 of 4 strings) Translation: account-invoicing-12.0/account-invoicing-12.0-account_invoice_fiscal_position_update Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-12-0/account-invoicing-12-0-account_invoice_fiscal_position_update/pt_BR/
Currently translated at 100.0% (4 of 4 strings) Translation: account-invoicing-13.0/account-invoicing-13.0-account_invoice_fiscal_position_update Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-13-0/account-invoicing-13-0-account_invoice_fiscal_position_update/nl/
Currently translated at 100.0% (4 of 4 strings) Translation: account-invoicing-13.0/account-invoicing-13.0-account_invoice_fiscal_position_update Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-13-0/account-invoicing-13-0-account_invoice_fiscal_position_update/es/
…xes after changed
…s without comments or notes.
… taxes when this was changed. Otherwise the taxes are not changed when change the fpos.
…fiscal position with a line without product and description
rov-adhoc
force-pushed
the
18.0-mig-account_invoice_fiscal_position_update
branch
from
849521c to
3d2c91b
Compare
Done |
|
Yes, but now you squashed my changes into the migration commit. That is not correct. The original commit should be kept. |
Fixes OCA#1896 This change causes the subtotal amount to remain the same when a fiscal position change triggers a tax update from tax included in price to tax excluded from price. Tests are added for conversion between tax excluded to tax included as well, but in this case the behaviour is not 'fixed'. The standard behaviour of Odoo in case of tax excl. to tax incl. is to adjust the subtotal amount. The same behaviour is present in the function that this module provides. It might not be what users expect, but it seems that Odoo does not support to preserve the subtotal amount in this case.
rov-adhoc
force-pushed
the
18.0-mig-account_invoice_fiscal_position_update
branch
from
3d2c91b to
7a3fef1
Compare
Hope now its ok |
|
This PR has the |
|
/ocabot merge nobump |
|
Sorry @legalsylvain you are not allowed to merge. To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons. If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the |
|
/ocabot merge nobump |
|
Hey, thanks for contributing! Proceeding to merge this for you. |
|
Congratulations, your PR was merged at 18ad772. Thanks a lot for contributing to OCA. ❤️ |
|
/ocabot migration account_invoice_fiscal_position_update |
|
This module is not needed anymore since 17.0: #1935 (comment) I think we should remove it. @StefanRijnhart @rov-adhoc @cav-adhoc |
|
@pedrobaeza Maybe it's a bit harsh to remove it, but maybe we can remove the code, set it to uninstallable and just note the obsolescense in the module description? This would not break automatic deploys where the module is already installed I think. |
|
OK, that seems the best path for those with the module installed. @rov-adhoc are you able to do it? |
|
@pedrobaeza @StefanRijnhart Hey! I’ve created the PR for this. Let me know if any changes are needed! |
No description provided.